Review: template-no-restricted-invocations

Compared with the original ember-template-lint no-restricted-invocations rule.

What looks good

• Comprehensive implementation covering all invocation types: MustacheStatement, BlockStatement, ElementNode, SubExpression, and ModifierStatement.
• Correct dasherize function that handles :: namespace separators (e.g., NestedScope::FooBar → nested-scope/foo-bar).
• Block param scope tracking with push/pop correctly prevents false positives on yielded names.
• Custom error message support via the { names, message } object config format matches the original.
• component helper detection works for both mustache and subexpression forms.
• Extensive test suite covering GJS and HBS modes, with tests for modifiers, nested subexpressions, component helper, block params, nested scopes, and custom messages.

Potential issues

1. parseConfig called but result of false not fully handled: When parseConfig returns false, the create method returns {} (no-op). However, when config is true, it returns [] (empty array). The original rule throws on invalid config; here passing true with no items would silently do nothing. This is fine behavior but differs slightly from the original which would also accept true with an error. Minor difference.

2. Original uses this.isLocal(node) for all node types: The original checks this.isLocal(node) at the top of the unified checkDenylist function, which leverages the base class's scope tracking. The PR re-implements scope tracking manually with blockParamScopes. This is necessary since ESLint rules don't have the isLocal base class, but worth verifying the scope tracking is complete:

   • GlimmerBlockStatement pushes node.program.blockParams ✅
   • GlimmerElementNode pushes node.blockParams ✅
   • Both have :exit handlers that pop ✅
   • The isBlockParam check is applied before restriction checks ✅

   This looks correct.

3. Original validates config items with isDasherizedComponentOrHelperName: The original enforces that config items are valid dasherized names. The ESLint version relies on schema validation but the schema only checks type: 'string' -- it doesn't enforce the dasherized naming convention. This means invalid names like "Foo Bar" would be accepted by the schema. Minor difference, but could be worth noting in docs.

4. Error message format differences for SubExpression: The original uses {{name}} format for the display name of all non-ElementNode types. The PR uses different formats: (name) for SubExpression, {{name}} for Mustache/Block/Modifier. Looking at the tests, SubExpression errors show '(foo)' and '(component "foo")' which matches the original's behavior of using parentheses for subexpressions. Actually, the original source shows it uses {{name}} for all non-ElementNode types. The PR diverges by using (name) for subexpressions. This seems like a deliberate improvement to better represent the actual syntax.

5. ElementNode modifier checking: The PR checks node.modifiers inside GlimmerElementNode visitor. This is a good approach since modifiers aren't separate visitors in the Glimmer AST -- they're properties of the element node. However, the PR also has a separate GlimmerModifierStatement visitor. This could potentially result in double-reporting if both fire for the same modifier. Worth verifying that GlimmerModifierStatement is actually emitted by the parser, or if modifiers are only accessible via node.modifiers on elements.

6. Test comprehensiveness: Excellent coverage. The HBS tests mirror all the patterns from the original test file including nested subexpressions, component helper variants, and custom messages.

Minor

• /* eslint-disable */ comments at top and bottom are a pragmatic choice for rule files that need different style.
• The originallyFrom metadata is a nice touch for traceability.

Very solid port with thorough test coverage. The main concern is potential double-reporting of modifiers (#5).

🤖 Automated review comparing with ember-template-lint source